Repository navigation
fix(fireworks_ai): resolve tool_choice and reasoning support for short model names - #39763
Conversation
…del names Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
|
Greptile SummaryFixes Fireworks capability detection for short model names by combining normalized exact cost-map lookup with existing generic capability resolution. Adds focused regression coverage for short-name tool-choice and reasoning support, plus generic reasoning fallback preservation Confidence Score: 5/5The PR appears safe to merge, with focused capability lookup changes and regression coverage for both corrected and preserved behavior No actionable correctness, security, compatibility, or repository-rule issue remains in the reviewed changes
|
| Filename | Overview |
|---|---|
| litellm/llms/fireworks_ai/chat/transformation.py | Resolves tool-choice and reasoning capabilities through exact normalized Fireworks metadata while retaining existing generic fallbacks |
| tests/test_litellm/llms/fireworks_ai/chat/test_fireworks_ai_chat_transformation.py | Adds focused regression tests for short Fireworks model names and generic reasoning fallback behavior |
Reviews (2): Last reviewed commit: "style(fireworks_ai): format capability f..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
|
|
||
| # Only add reasoning params for models that support it | ||
| if supports_reasoning(model=model, custom_llm_provider="fireworks_ai"): | ||
| if self._get_model_cost_capability_exact(model=model, capability="supports_reasoning"): |
There was a problem hiding this comment.
🟡 Custom reasoning parameters get rejected
Custom model names matching known reasoning models now fail _get_model_cost_capability_exact. Their reasoning parameters are rejected or silently dropped instead of reaching Fireworks.
Prompt for agents
Restore reasoning capability handling for custom and suffixed Fireworks model names while preserving short-name resolution. In litellm/llms/fireworks_ai/chat/transformation.py, the old supports_reasoning call could obtain supports_reasoning from FireworksAIConfig.get_provider_info, which uses _get_model_cost_capability's hyphen-boundary fallback. The replacement uses only _get_model_cost_capability_exact, so names such as accounts/myorg/models/custom-glm-5p1 no longer advertise reasoning parameters. Choose a lookup that resolves default-account short names without removing the prior fuzzy reasoning support, and add a regression test for a custom or suffixed reasoning model.
Was this helpful? React with 👍 or 👎 to provide feedback.
… tool_choice Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit dad1b13. Configure here.
41c8c2f
into
litellm_internal_staging
TLDR
Problem this solves:
fireworks_ai/deepseek-v4-pro-0813rejectstool_choicewith UnsupportedParamsErroraccounts/fireworks/models/prefixtool_choiceand reasoning gates did an exact lookup, so the short name missedHow it solves it:
tool_choiceand reasoning params on_get_model_cost_capability_exactfirst, which already tries thefireworks_ai/accounts/fireworks/models/<name>keysupports_tool_choice/supports_reasoningresolvers so their bare-key and provider-info fallbacks keep working (a sweep of all 318 Fireworks cost-map keys shows 16 models gaintool_choice, none lose anything)User Flow
Before: a developer using opencode against the gateway gets an error on every tool call turn
model: fireworks_ai/deepseek-v4-pro-0813inmodel_listtoolsand"tool_choice": "auto"litellm.UnsupportedParamsError: fireworks_ai does not support parameters: ['tool_choice']and the fallback chain fails tooAfter: the same request reaches Fireworks and returns a normal completion
model: fireworks_ai/deepseek-v4-pro-0813inmodel_listtoolsand"tool_choice": "auto"Relevant issues
Reported in an internal Slack thread by a teammate running opencode against the gateway
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
QA'd with the real end-user client: opencode 1.18.29 TUI, driven interactively in tmux, against a live proxy booted with
--num_workers 2(no database) making real Fireworks calls. Same scratch project, same opencode.json (only the port differs), same prompt on both legs; the only difference is the commit the proxy was booted fromProxy config:
Proxy:
.venv/bin/python -m litellm.proxy.proxy_cli --config config.yaml --port <port> --num_workers 2opencode.json in the scratch project (README.md, hello.py, opencode.json):
{"$schema":"https://opencode.ai/config.json", "provider":{"litellm":{"npm":"@ai-sdk/openai-compatible","name":"litellm", "options":{"baseURL":"http://localhost:<port>/v1","apiKey":"sk-1234"}, "models":{"deepseek-v4-pro-0813":{"name":"deepseek-v4-pro-0813"}}}}, "model":"litellm/deepseek-v4-pro-0813","permission":"allow"}Before (2e73400, merge base)
tmux new -s qa,cdinto the scratch project, runopencodeList the files in this directory and read README.md, press Enter"POST /v1/chat/completions HTTP/1.1" 400 Bad Request(the tool-call request) followed by one200 OKfor opencode's session-title request, which carries no toolsAfter (dad1b13)
tmux new -s qa,cdinto the same scratch project, runopencodeList the files in this directory and read README.md, press Enter"POST /v1/chat/completions HTTP/1.1" 200 OK, no UnsupportedParamsErrorOther unified endpoints (same two proxies, curl)
The gate lives in the shared param validation, so the same short name was driven through the other two unified endpoints and a streaming chat request on both proxies,
Hbeing-H "Authorization: Bearer sk-1234" -H "content-type: application/json"Before (2e73400, port 23505), each request:
After (dad1b13, port 26389), each request:
Observed alongside the fix:
Type
🐛 Bug Fix
Caveats (if any)
Low
tool_choice, same as before_translate_chat_template_kwargsstill uses the genericsupports_reasoningalone; it does not hit the short-name miss for this model, left as islitellm.utils.supports_tool_choice()andlitellm.get_model_info()still miss Fireworks short names (/model/infoshowssupports_tool_choice: nullfor the short name on both trees); only the param gate resolves them here. Pre-existing and left alone, since fixing it means changingget_provider_infooutput for every Fireworks model, which is more surface than this fix should carryci/circlecijobs are red on this PR only because the branch predates staging commits dbf8fe0 and a5a7867 (test: repair four chronically failing CI testsand its review follow-up):litellm_router_unit_testingfailstest_router_index_management.py::test_no_linear_scans_in_router(the flaggedrouter.pyloops exist at the merge base 2e73400 and staging allowlisted them),proxy_store_model_in_db_testsfailstest_openai_error_handling.py::test_missing_model_parameter_curl[chat](staging relaxed its assertion to HTTP 400 plus a non-empty message), andbuild_and_testfailstests/test_models.py::test_model_group_info_e2e(staging rewrote the wildcard assertion) plus the Rust harnesstest_should_derive_ocr_mapping_status_from_live_tests(staging skips it). Staging pipeline 89088 fails all of them the same way and 89097, 89104, and 89115 pass; this PR touches none of those filesresponses-caching-types(3.10, 3.11) failstests/test_litellm/caching/test_redis_cache.py::test_event_loop_stall_timeout_burst_keeps_breaker_closed(a stall-timing flake) andproxy-infra(3.10) failstest_http_parsing_utils.py::TestNumericFormFields::test_qualifiers_and_optionality_are_unwrapped, tracked in fix(proxy): strip every TypedDict qualifier before numeric form-field detection #39780; all 33 required checks passFinal Attestation
Link to Devin session: https://app.devin.ai/sessions/2fccaa5a6fbd40f9b38b0433ecfc6b2e
Open in Devin Desktop: https://app.devin.ai/desktop/session/2fccaa5a6fbd40f9b38b0433ecfc6b2e?variant=devin
Requested by: @krrish-berri-2
Note
Low Risk
Narrows param validation for Fireworks only by fixing cost-map lookup; behavior change is allowing params that were incorrectly blocked, with tests and no reported capability regressions.
Overview
Fixes false
UnsupportedParamsErrorwhen callers use short Fireworks model ids (e.g.fireworks_ai/deepseek-v4-pro-0813) while capability flags in the cost map only exist under theaccounts/fireworks/models/key.get_supported_openai_paramsnow treatstool_choiceand reasoning-related params (reasoning_effort,reasoning_history,thinking) as supported when_get_model_cost_capability_exactfinds the flag on an alternate cost-map key, or when the existingsupports_tool_choice/supports_reasoninghelpers still say yes—so short names inherit the prefixed entry without dropping prior fallbacks.Adds regression tests for the short-name case and for generic reasoning fallback on models like
glm-5p3-flash.Reviewed by Cursor Bugbot for commit dad1b13. Bugbot is set up for automated code reviews on this repo. Configure here.
/live-pr-risk at dad1b13
Breaking: none. Backward incompatible: the
get_supported_openai_paramslist for 16 Fireworks short names (deepseek-v3p1,deepseek-v3p1-terminus,deepseek-v3p2,deepseek-v4-pro-0813,firefunction-v2,glm-4p5,glm-4p5-air,glm-4p6,glm-5p3,glm-5p3-flash,inkling,kimi-k2-instruct,kimi-k2-instruct-0905,kimi-k2-thinking,llama-v3p1-405b-instruct,mixtral-8x22b-instruct-hf) grows bytool_choice; a sweep over all 317fireworks_ai/cost-map keys and 575 name shapes shows no key losing a param and no reasoning-param delta on any shape. Regression risk: none found; the only dependents are the provider dispatch inget_supported_openai_params.py,map_openai_params, andget_optional_paramsvalidation, all driven live on both trees through/v1/chat/completions(tool_choice required, named, streaming),/v1/messages,/v1/responses,/utils/supported_openai_params, plusreasoning_effortandthinking(identical 200s on both sides)._get_model_cost_capability_exactalready gatedsupports_visionthe same way, no subclasses overrideFireworksAIConfig, and the othersupports_tool_choice(callers (azure_ai, bedrock converse, gpt_5) are untouched. Fireworkstests/llm_translation/andtests/local_testing/hits pass on the head tree. Merge-ref drift (141 staging commits, 5 touching Fireworks off-peak pricing): re-ran S2 (chat tool_choice required), S6 (/v1/messages), S7 (/v1/responses), and S8 (streaming chat) on a proxy booted from origin/litellm_internal_staging 59d42d3 merged with dad1b13 (76537482cf, 2 workers): all 200 with a get_weather tool call, 24 stream chunks with 6 tool_calls deltas, and /utils/supported_openai_params lists tool_choice